Skip to content

fix(tools): read preprocessor branch combinations lazily in import_audit - #11622

Merged
adriadam10 merged 2 commits into
boykopovar:mainfrom
BunnyFl:fix/import-audit-lazy-branch-variants
Oct 28, 2026
Merged

adriadam10 merged 2 commits into
boykopovar:mainfrom
BunnyFl:fix/import-audit-lazy-branch-variants

Conversation

@BunnyFl

@BunnyFl BunnyFl commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

What

Since #9509, poll in core/libs/prx/libkernel/Socket/src/Udp.cpp has more than 256 preprocessor branch combinations. branch_variants builds every combination of every function up front, so each tools/import_audit.py run on main stops with FAIL: more than 256 preprocessor branch combinations in one function before it audits anything. poll is the only function in core/libs/prx over the limit.

branch_variants now produces the combinations one at a time, in the same order as before. The alternatives of nested #if groups are also produced only when needed, and the limit counts only complete combinations. conditional_stubs stops at the first combination that does not reach a stub call, so a function fails only when the audit has to read more than 256 of its combinations. always_stub_calls still reads all of them.

Tested

Windows 11, Python 3.14.7.

  • python -m unittest test_import_audit in tools/tests: 154 tests OK (3 skipped). The new test uses nine independent #ifdef blocks (512 combinations). It covers a function without a stub call; a function whose stub call sits in the last #ifdef and is reported as partial; a function whose first branch of an outer #ifdef holds the nine blocks, with the stub call in #else, also reported as partial; and two functions that reach a stub in every combination (flat and nested), which still fail. The test fails on main with the error above, and its nested case also fails on this PR's first head 94d991e.
  • 60,000 randomly generated function bodies with nested #if/#ifdef/#ifndef/#elif/#else (three seeds): every body gives the same combinations in the same order as main's branch_variants, or fails the limit in both. In a fourth set of 20,000, all 2,087 bodies with 257 to 4,096 combinations give main's first 256 combinations in main's order (with its limit raised to 4,096), and the 257th fails.
  • Every function body in core/libs/prx (28,805 on 74acfad; 28,498 on c8809e8, main just before fix(libkernel): report urgent data in poll on macOS #9509): the new branch_variants returns the same strings in the same order as main's. The only body that fails on main is poll.
  • source_stub_classes: identical to main's on c8809e8 (6,337 stubs, 451 partial). On 74acfad it is identical to main's code with BRANCH_VARIANT_LIMIT raised to 4096 (6,349 stubs, 470 partial).
  • ASTRO BOT (PPSA21567) registry: main's tool on 74acfad exits 2 with the error above, and this PR's tool completes the audit. With --source set to c8809e8, both tools write identical text and identical --json output.

Because of this error, the audits in #11607, #11608 and #11609 were run with --source set to c8809e8.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found one issue in the lazy expansion path. In branch_variants.expand, each conditional group eagerly materializes choices with a list comprehension before the outer iterator can yield (tools/import_audit.py, around lines 542–544). With nine nested two-way #ifdef groups inside the first branch of an outer conditional, next(branch_variants(body)) raises AuditError: more than 256 preprocessor branch combinations before yielding even the first variant. I reproduced this against the PR head. That first branch can already let conditional_stubs stop as soon as it sees a variant without a stub call, so the current implementation still rejects some functions that need only one combination to classify. Please keep nested choices lazy as well, with a regression test where an early branch is enough to classify the function and a later nested branch contains more than 256 combinations.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@BunnyFl

BunnyFl commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

@MarcZX10 Thanks, you're right: each group's alternatives were still built in full before the first combination came out. 3d2aebfa03 keeps nested groups lazy as well. Each group's alternatives are now produced only when the walk reaches them, in the same order as before, and the limit counts only complete combinations.

The test now includes your layout: an outer #ifdef whose first branch holds the nine #ifdef blocks, with the stub call in #else. It is reported as partial, and the same test fails on 94d991e03a with more than 256 preprocessor branch combinations. A nested function that reaches a stub in every combination still fails. I also compared the new branch_variants with main's on 60,000 random nested bodies: every one gives the same combinations in the same order, or fails the limit in both. The description has the updated results.

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed exact head 3d2aebfa0313a518a43530f2a9da777f196cb330. choices() now expands nested alternatives lazily, and limited() counts complete variants only as the consumer requests them. The regression test covers both early classification without exhausting nested branches and the case where all combinations must be read and the 256 limit still applies. The author also reports randomized equivalence checks against main. I independently ran python3 -m unittest tools.tests.test_import_audit in a detached checkout: 154 tests passed, 1 skipped; tools/check_conventions.py --base upstream/main --head 3d2aebfa0313a518a43530f2a9da777f196cb330 and git diff --check upstream/main...upstream/pr/11622 pass. No remaining actionable finding.

@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request Shared files not compared
PR 6171 tools/import_audit.py
tools/tests/test_import_audit.py
PR 7454 tools/import_audit.py
tools/tests/test_import_audit.py
PR 11581 tools/import_audit.py
tools/tests/test_import_audit.py

@THEMANnt THEMANnt left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 3d2aebfa0313a518a43530f2a9da777f196cb330. Approve: branch combinations are generated lazily, including nested alternatives, while the 256-result limit still applies when all combinations must be consumed.

Windows/Python 3.11.9: 151 import-audit tests passed, 3 skipped, 154 total. An independent control against author base 74acfad6 produced identical ordered output for all 256 eight-branch combinations. For nine branches, including a nested case, the old code fails before yielding the first combination; this head yields the first and still raises when asked for the 257th. This establishes both the lazy behavior and the retained limit.

Hosted Windows, Linux and both macOS relinker jobs ran the registered import-audit suite on merges containing this head. Product jobs were skipped. The focused validation covers the Python audit, not a full application or game run.

@adriadam10 adriadam10 self-assigned this Oct 27, 2026
@JJenkx

JJenkx commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

A real-world check of tools/import_audit.py from this PR's head 3d2aebfa03, run against current main 62b3ba2522 sources.

Setup: converted import registries (*.registry.json from relinker --windows --registry) of 22 titles, read from local copies of their dumps. --libs is a Windows Release build of main 3fea2a937b. Windows 11, Python 3.15.1. Only tools/import_audit.py was swapped in; the other sources stay at main.

  • main's import_audit.py: all 22 runs stop with FAIL: more than 256 preprocessor branch combinations in one function, before auditing anything.
  • This PR's import_audit.py: all 22 runs complete and print the per-class counts, with no FAIL line. For example, PPSA19591 gives 1074 implemented, 29 partial, 5 stub and 6 absent imports.

I did not compare classifications line by line with an older main. This only shows that the tool runs again on real registries.

AI-assisted: yes (Claude Code).

@johnohhh1

Copy link
Copy Markdown
Contributor

Independent Linux confirmation of your 3d2aebfa0313a518a43530f2a9da777f196cb330, tested in a private merge with main 2ee8e9751a06bd4cb7edb8d17f433d56a39a7025 (bd494620dc498c979601c8048e46f726d5e02308).

Ubuntu 26.04.1 x86-64, Python 3.14.4:

  • Full import-audit suite: 155 tests run, 154 pass, one Windows-only skip. Conventions and whitespace pass.
  • Unchanged main's scan of its actual core/libs/prx stops at poll_nid_postfix in libkernel/Socket/src/Udp.cpp with the 256-combination refusal. This candidate completes the scan. All 7,225 stub entries and 524 partial entries match exactly against the isolated reference using unchanged main code with a process-local limit of 4,096. The limit change is only a validation control, not the production fix. The complete normalized result SHA256 is e05bc24d738f3ac5a49f0278b67b54d8f1b25b5e4c2fa61d17e59cca19137df4.
  • Replayed both saved Middle Ages Ping Pong PPSA28319 datasets (October 17/23), using the actual pinned-current-main classification sources, all five registries, three module directories and hash-checked frozen runtime libraries. Main exits 2 before reporting; this candidate completes each audit and exits 1 with the same separate four-import module-identity problem tracked by fix(tools): recognize guest module identities in import audit #6171. That remaining exit 1 is expected and does not mean the source-scan correction failed.
  • A separate private integration with the reviewed identity/path corrections and fix(tools): accept PT_DYNAMIC data after DT_NULL in import_audit #11499/fix(tools): require a module suffix on both names when matching aliases #11581 reaches exit 0 on those saved datasets and retains the same complete source classifications. Its full tools suite runs 1,367 tests: 1,342 pass, 25 environment/platform skips. These combined results do not describe the standalone fix(tools): read preprocessor branch combinations lazily in import_audit #11622 tree.

These are fresh Linux Python/static-audit checks using archived converted assets, not new title, native-game, Windows or console execution. The runtime libraries retain their original October 17/23 build pins; no PR branch has been changed.

AI-assisted validation and reporting: OpenAI Codex.

@Amoenus

Amoenus commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

@BunnyFl Independent Windows confirmation of 3d2aebfa0313a518a43530f2a9da777f196cb330, cleanly merged with current main f759e165d9cc3088b196f9b544e038687003fef4 in validation composition 3a1e9245cd43. Windows 11 Enterprise 26200, Python 3.13.15.

  • Import-audit, progress, progress-wrapper and NID-name suites: 256 tests run, 254 pass, two POSIX-permission skips. Conventions and whitespace pass.
  • Keeping your regression while substituting main's production module fails with the expected more than 256 preprocessor branch combinations error. The candidate passes, including its flat/nested always-stub cases that retain the refusal.
  • The candidate completes classification of the actual current-main core/libs/prx: 7,232 stub-or-partial entries, 528 partial. It also completes our separately pinned Sonic integration 879d6d9ba22f45932a9cebc35bdc682d9fb7b388: 7,231 entries, 527 partial. Complete normalized classification hashes and timings are in the receipt.

Reproduction script, test logs, negative control and exact pins. These are public-source and synthetic-fixture checks, not fresh game execution, a full import-registry audit, or a claim that classifications were compared with an older main. No branch limit was raised. I encountered the same poll_nid_postfix failure independently; your existing PR covers it, so no competing PR is being submitted.

AI-assisted validation and reporting: OpenAI Codex.

@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the lazy expansion (nested alternatives and the complete-combination limit), that variant_calls consumers (extend, short-circuiting all) read it once, and test_import_audit (155 passed, 1 skipped); merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants